Zend/zend_compile.h: use uint32_t type for zend_op_array.last_var#21543
Zend/zend_compile.h: use uint32_t type for zend_op_array.last_var#21543Girgias wants to merge 1 commit intophp:masterfrom
Conversation
github-actions
bot
added
Category: Engine
Extension: opcache
Category: Optimizer
ABI break
labels
This comment was marked as off-topic.
This comment was marked as off-topic.
| uint32_t level = 1 + trace_buffer[0].level; | ||
| int idx, len, i, v, vars_count, call_level; | ||
| int len, v; | ||
| uint32_t idx, i, vars_count, call_level; |
There was a problem hiding this comment.
Seems quite inconsistent, the same variables are declared many times in the JIT files, and i could be local for pretty much all of them.
There was a problem hiding this comment.
I'm happy to make the i variable local, wasn't sure if I should or not.
| /* We iterate the variables backwards, so we can eliminate sequences like INIT_ROPE | ||
| * and INIT_ARRAY. */ | ||
| for (i = ssa->vars_count - 1; i >= op_array->last_var; i--) { | ||
| for (i = ssa->vars_count - 1; i >= (int)op_array->last_var; i--) { |
There was a problem hiding this comment.
😕 This this warn? That's pretty error prone.
There was a problem hiding this comment.
Was breaking opcache, not sure why.
There was a problem hiding this comment.
Because C integer coercion semantics are wonky. int gets coerced to unsigned, which breaks the >= check when op_array->last_var == 0.
| int i; | ||
| for (i = 0; i < op_array->last_var; i++) { | ||
| if (zend_string_equals(op_array->vars[i], var_name)) { | ||
| for (uint32_t j = 0; j < op_array->last_var; j++) { |
There was a problem hiding this comment.
I don't understand the switch in variable name here. There's no surrounding i?
There was a problem hiding this comment.
I was getting a shadowing warning. I'll double check.
3 participants
No description provided.